Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement TryFrom<T> for u5 for all standard integer types #92

Merged
merged 2 commits into from
Mar 4, 2023

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Mar 3, 2023

Patch 1 is preparatory clean up.

From the commit log of patch 2:

Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid
value encountered while converting to a `u8`. This is buggy because we
cast to the `u8` to stash the value so we loose valuable bits.

Implement `TryFrom` by doing:
- Add an `Error::Overflow` variant.
- Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls
- Add a new `TryFromError` and nest it in `Error::TryFrom`
- Implement `TryFrom<T> for u5` impls for all standard integer types.

Inspired by issue #60 and the stdlib macros of the same name.

Please note, there is expected error work to come soon on this crate, can we keep perfect error code out of scope for this PR please (obviously if it has gaping problems then yell at me)? Note that the same derives are used here as for Error even though I think they are wrong.

Done as part of stabilising (#60).

Add a `use Error::*` line to use error variants locally, while we are at
it make usage uniform.
Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid
value encountered while converting to a `u8`. This is buggy because we
cast to the `u8` to stash the value so we loose valuable bits.

Implement `TryFrom` by doing:
- Add an `Error::Overflow` variant.
- Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls
- Add a new `TryFromError` and nest it in `Error::TryFrom`
- Implement `TryFrom<T> for u5` impls for all standard integer types.

Inspired by issue rust-bitcoin#60 and the stdlib macros of the same name.
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 25462c9

@apoelstra apoelstra merged commit 27bfcba into rust-bitcoin:master Mar 4, 2023
@tcharding tcharding deleted the 03-03-try-from branch March 5, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants